Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sofle Shield: Add underglow support and increase encoder sensitivity #1188

Closed
wants to merge 2 commits into from
Closed

Sofle Shield: Add underglow support and increase encoder sensitivity #1188

wants to merge 2 commits into from

Conversation

infused-kim
Copy link
Contributor

@infused-kim infused-kim commented Mar 26, 2022 •
edited
Loading

This PR adds underglow support to the sofle keyboard. I used the kyria shield as a reference.

It's been tested on a sofle choc, but the pins are the same on the sofle RGB and there should be no issues.

Board/Shield Check-list

  • This board/shield is tested working on real hardware
  • Definitions follow the general style of other shields/boards upstream (Reference)
  • .zmk.yml metadata file added
  • Proper Copyright + License headers added to applicable files (Generally, we stick to "The ZMK Contributors" for copyrights to help avoid churn when files get edited)
  • General consistent formatting of DeviceTree files
  • Keymaps do not use deprecated key defines (Check using the upgrader tool)
  • &pro_micro used in favor of &pro_micro_d/a if applicable
  • If split, no name added for the right/peripheral half
  • Kconfig.defconfig file correctly wraps all configuration in conditional on the shield symbol
  • .conf file has optional extra features commented out
  • Keyboard/PCB is part of a shipped group buy or is generally available in stock to purchase (OSH/personal projects without general availability should create a zmk-config repo instead)

caksoylar
caksoylar reviewed Mar 28, 2022
View reviewed changes
app/boards/shields/sofle/sofle.dtsi Outdated
@@ -49,7 +49,7 @@ RC(3,0) RC(3,1) RC(3,2) RC(3,3) RC(3,4) RC(3,5) RC(4,5) RC(4,6) RC(3,6) RC(3,7)
label = "LEFT_ENCODER";
a-gpios = <&pro_micro 21 (GPIO_ACTIVE_HIGH | GPIO_PULL_UP)>;
b-gpios = <&pro_micro 20 (GPIO_ACTIVE_HIGH | GPIO_PULL_UP)>;
resolution = <4>;
resolution = <2>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this depends on the encoder hardware itself and not the shield. IMO it should be set the same for all shields (to the most common value if possible) then the user should override it in their config if their encoder is different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize that. I just noticed my encoder was skipping steps.

No problem, I can remove that commit if that helps getting the changes merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this now.

This was referenced Mar 31, 2022
Open
Open
@infused-kim infused-kim force-pushed the my-changes/sofle-improvements branch from cd4f9fb to 761a0aa Compare April 29, 2022 10:27
@infused-kim
Copy link
Contributor Author

I have removed the encoder sensitivity commit as per @caksoylar's suggestion and updated the underglow settings for zephyr 3.

Is there anything else that is necessary to be done to get this merged?

@Nicell Nicell added the shields PRs and issues related to shields label Jun 26, 2022
@Nicell
Copy link
Member

Nicell commented Jun 26, 2022

@infused-kim It looks like this branch is failing to build ATM: https://github.com/infused-kim/zmk/runs/6226559733
Is this just because it's so far behind main it doesn't have the updates requiring color mapping?

@infused-kim infused-kim force-pushed the my-changes/sofle-improvements branch from 761a0aa to 297ff8b Compare October 8, 2022 23:45
@Cringely
Copy link

Would love for this to be merged, thanks for the effort @infused-kim!

@autoferrit
Copy link

I recently acquired this board. I would love to see this merged. I am new to C and ZMK but would like to know if there is anything I can do to help get this merged in.

@SethMilliken SethMilliken mentioned this pull request Jul 10, 2023
Merged
@SethMilliken
Copy link
Contributor

I've run across a few more instances on various Discord servers of people having issues enabling underglow on their Sofle. Getting these changes landed seems like very low-hanging fruit with no downside. I would be happy to update this PR with the 3.2 pinctrl changes if that helps.

@SethMilliken
Copy link
Contributor

These changes (updated for Zephyr 3.2) have now been merged! This PR can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caksoylar caksoylar caksoylar left review comments

Assignees
No one assigned
Labels
shields PRs and issues related to shields
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@infused-kim @Nicell @Cringely @autoferrit @SethMilliken @caksoylar @petejohanson